Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Switch to Go backend; modify backend request pattern. #235

Closed
wants to merge 8 commits into from
Closed

Switch to Go backend; modify backend request pattern. #235

wants to merge 8 commits into from

Conversation

krashanoff
Copy link
Contributor

Ahoy! This is a PR to switch out our backends from JS to Go. Here's a breakdown of the changes:

  • Easier to follow calls to the backend in our modals and fetch.js.
    • Most of the endpoints use query parameters.
  • Use HTTP response codes for validating responses as opposed to the ok field.
  • Addition of mostRecentProgramID prop to the Main component.
    • Stores the document ID of the most recent program of an user as a prop. It is used for
  • Update updateProgram behavior
    • Update programs one by one instead of via array. We are only ever updating a single program at a time, so having it passed as an array didn't make much sense.
  • Change our deleteProgram call to use both user ID and program ID (uid, pid) for program deletion instead of the old uid, docID, and name system.
    • This led to the creation of the pid prop in ConfirmDeleteModal, which is used when the modal makes the call to the backend.
  • Tweak our updateUserData calls slightly (everything is sent as an object).
  • Pass down a sketchCount prop to our CreateSketchModal used when creating a new sketch.

This PR is presently blocked on account of this PR which updates the Go backend to work with the frontend changes made here. I'll unblock it on merge of said PR.

Stay healthy everyone and have a great spring break!

@krashanoff krashanoff added the blocked Blocked (for some reason or another) label Mar 20, 2020
krashanoff
krashanoff commented Mar 20, 2020
View reviewed changes
src/components/Sketches/index.js
Comment on lines +172 to +176
pid={
Array.from(this.props.programs)[this.state.selectedKey]
? Array.from(this.props.programs)[this.state.selectedKey].uid
: ""
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a huge fan of how I made the changes to this file, but these felt the most natural without modifying much else. Thoughts?

@mattxwang
Copy link
Member

I'll give a more formal review after I finish my final, but just some quick notes:

  • merging this & the view-only branch is going to be a bit of a fiasco, especially on the testing end
  • we need to change the server addresses and/or npm actions to account for the new Go heroku URL
  • looks like you've slightly changed the props structure for some components, so I'll have to investigate

I think it would probably be wise to incrementally implement these changes, i.e. allow dev to use the Go features but maintain "backwards compatibility" for production until we're 100% sure of the changes. Again, will discuss further in a bit.

@krashanoff
Copy link
Contributor Author

merging this & the view-only branch is going to be a bit of a fiasco, especially on the testing end

I'm not opposed to simply basing my changes directly off of the view-only branch if that makes things easier on us. Between this PR and the view-only one, it's going to be an interesting time merging these all.

we need to change the server addresses and/or npm actions to account for the new Go heroku URL

Will do.

looks like you've slightly changed the props structure for some components, so I'll have to investigate

Here's the props I have added with brief descriptions:

  • pid prop to ConfirmDeleteModal, which is the document ID of the program being deleted (used in the request to the backend).
  • sketchCount prop to CreateSketchModal, the number of sketches currently listed (used when creating a new sketch and assigning an user's mostRecentProgram after creation).
  • mostRecentProgramID to MainContainer, the document ID of the most recent program (used for saving the current program).

@krashanoff krashanoff mentioned this pull request Apr 8, 2020
Merged
@krashanoff
Copy link
Contributor Author

Superseded by #253.

@krashanoff krashanoff closed this Apr 8, 2020
@krashanoff krashanoff deleted the go-switch branch April 8, 2020 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
blocked Blocked (for some reason or another)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@krashanoff @mattxwang